-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make Algae.Maybe.new(nil) return a Nothing #27
Conversation
Uh, a Now |
@icidasset because EDIT: I actually kind of like the |
Thanks @expede ! I've implemented your idea. |
lib/algae/maybe.ex
Outdated
if value == nothing_value, | ||
do: Nothing.new(), | ||
else: Just.new(value) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two very minor nitpicks:
Nothing
missing in @spec
@spec new(any(), [nothing: any()]) :: Just.t() | Nothing.t()
(Also may as well add the expected key ☝️)
Pattern Matching
Suuuuper optional, but have you considered the following?
def new(nope, [nothing: nope]), do: Nothing.new()
def new(value, _), do: Just.new(value)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@expede Thanks! I didn't know you could match variables with pattern matching, neat! Spec is fixed as well.
Why not something more like this for @spec new(err, [nothing: err]) :: Nothing.t(err)
@spec new(v, [nothing: err]) :: Just.t(v) when v != err And adjust EDIT: Although the above would only work if err and value types are fully distinct, which might be enough for the top level new but would not be for the distinct cases, which is not an issue there anyway as they don't reference each other... Need tests... |
@OvermindDL1 Yeah, the automagic type generation could use some work. At the time (way back in the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks awesome @icidasset — thanks!
Was this what you had in mind?☺️
Can also write some more tests if you want me to